[bugfix] fix multimodal mtp#21
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modifies src/mcore_bridge/model/gpt_model.py to add a None check for input_ids before calling split_cp_inputs, which prevents potential crashes during multimodal inference. The review points out a potential logical inconsistency: if input_ids are indeed required by MTP, as indicated by the new code comment, then the subsequent MTP processing block might still fail if input_ids is None but labels are present. It is suggested to either add a similar input_ids check to the MTP block or clarify its expected behavior.
| if self.config.is_multimodal and self.config.context_parallel_size > 1 and input_ids is not None: | ||
| # input_ids is required by MTP. |
There was a problem hiding this comment.
The addition of the input_ids is not None check prevents a crash in split_cp_inputs when input_ids are missing (e.g., during inference with decoder_input provided). However, the comment on line 403 states that input_ids is required by MTP. If this is the case, the MTP block starting at line 406 might still fail if input_ids is None while labels are present. It would be more robust to also check for input_ids at line 406 or clarify if MTP can indeed function without them.
1 participant
No description provided.